-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add runtime checks for use of SIMD implementations and stubs functions if not available. #1
Conversation
@@ -592,4 +592,23 @@ int64_t bshuf_untrans_bit_elem_altivec(const void* in, void* out, const size_t s | |||
return count; | |||
} | |||
|
|||
#endif /* defined(__ALTIVEC__) */ | |||
|
|||
const bool is_bshuf_altivec = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used naming consistent with the functions.
The naming (like the SIMD suffix) could be made consistent between bitshuffle and shuffle functions
#include "bitshuffle-avx512.h" | ||
#include "bitshuffle-avx2.h" | ||
#include "bitshuffle-sse2.h" | ||
#include "bitshuffle-generic.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved those includes outside of #if defined...
to be consistent with other SIMD implementations
@@ -22,13 +22,13 @@ | |||
|
|||
#include "bitshuffle-neon.h" | |||
#include "bitshuffle-generic.h" | |||
#include <stdlib.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move stdlib.h
include outside of #if defined
, it is also needed for abort
@@ -91,7 +91,8 @@ typedef enum { | |||
|
|||
/* Detect hardware and set function pointers to the best shuffle/unshuffle | |||
implementations supported by the host processor. */ | |||
#if defined(SHUFFLE_AVX2_ENABLED) || defined(SHUFFLE_SSE2_ENABLED) /* Intel/i686 */ | |||
#if (defined(SHUFFLE_AVX2_ENABLED) || defined(SHUFFLE_SSE2_ENABLED)) && \ | |||
(defined(__i386__) || defined(__x86_64__) || defined(_M_IX86) || defined(_M_X64)) /* Intel/i686 */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the check of architecture since for universal2
build this part which looks to be x86 only was compiled for arm as well (since SHUFFLE_AVX2_ENABLED and SHUFFLE_SSE2_ENABLED were defined)
@@ -296,7 +297,7 @@ return BLOSC_HAVE_NOTHING; | |||
static shuffle_implementation_t get_shuffle_implementation(void) { | |||
blosc_cpu_features cpu_features = blosc_get_cpu_features(); | |||
#if defined(SHUFFLE_AVX512_ENABLED) | |||
if (cpu_features & BLOSC_HAVE_AVX512) { | |||
if (cpu_features & BLOSC_HAVE_AVX512 && is_shuffle_avx2 && is_bshuf_AVX512) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runtime check of the const bool
defined by each SIMD implementation
This PR builds on Blosc#622 and adds stubs for shuffle and bitshuffle functions when a given SIMD is not available at runtime.
This aims at not relying only on
SHUFFLE_*_ENABLED
macros to ensure a SIMD implementation is available, but also to rely on the effective SIMD macros (e.g.,__AVX2__
) while not compileshuffle.c
with SIMD enabled.To do so, this PR adds a
is_shuffle_*
(oris_bshuf_*
)const bool
to each implementation which is set at build time according to the SIMD macros.This
const bool
is checked at runtime to check that a given SIMD implementation is available.is_shuffle_*
andis_bshuf_*
are currently doing the same checks, so only one should be needed. I preferred to use both to have each source file declaring its availability.I've checked that it works with macos
universal2
builds.